Cluster-coherence preflight guard (#95) - #107
Conversation
…st#95) Signed-off-by: Ebad Shahid <[email protected]>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds a preflight cluster-coherence guard that verifies OpenCost and Prometheus observe the same cluster before running integration tests. It compares running-pod sets from both sources using Jaccard similarity, failing fast if the overlap is below a threshold (0.50), which prevents downstream tests from producing meaningless results against mismatched targets.
Changes:
- Added a Go test (
cluster_coherence_test.go) that queries both sources for running pods over a shared historical window and compares them via Jaccard overlap. - Added a bats wrapper (
test.bats) taggedpreflightso the main test step can exclude it via--filter-tags. - Updated the CI workflow to run the preflight guard as a dedicated step before the main integration tests, using tag-based exclusion to avoid double-running.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/integration/preflight/test.bats | Bats wrapper that invokes the Go preflight test with cache-busting (-count=1). |
| test/integration/preflight/cluster_coherence_test.go | Core coherence logic: env-var validation, pod-set fetching from both sources, Jaccard comparison with churn resampling, and fresh-cluster fallback. |
| .github/workflows/integration-testing.yaml | Adds a dedicated preflight step and excludes preflight-tagged tests from the main recursive bats run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Ebad Shahid <[email protected]>
|
The tests pass so we know the code works. I like the idea of having a fast-fail mechanism, saves us build time. My only suggestion is since you are interested in the number of common pods between prometheus and allocation, I would think about repurposing the https://github.com/opencost/opencost-integration-tests/blob/main/test/integration/query/count/allocation_running_pods_test.go this existing test. It does the exact same thing you are trying to do, calculates the number of pods per namespace (slight difference) and performs a trivial comparison. How about adding the guards and preflight steps to that check instead @ameijer ? |
Description
Closes #95 -> pkg/prometheus/client.go silently falls back to the demo Prometheus when the prometheus url is unset, and pkg/env/env.go falls back to localhost when OPENCOST_URL is unset.
The smoke tests (#74) check each endpoint is reachable, but not that both endpoints observe the same cluster.
This PR adds a preflight guard that runs before the rest of the suite. It:
PROMETHEUS_URLorOPENCOST_URLis unset (refusing thesilent fallback),
kube_pod_container_status_runningfrom Prometheus and/allocationaggregated by pod — and compares their running-pod sets,
__idle__,*-unmounted-pvcs) and requirescluster-wide overlap of at least 50% (same-cluster is ~90%+, different-cluster
is ~0%, so normal churn can't cross the line),
t.Fatals with both URLs and sample pods from each side.It reuses the shared-timestamp, churn-tolerance, and retry from the pod-count test (#94).
Changes
test/integration/preflight/cluster_coherence_test.go(new) — the guarditself: env-var fail-fast, shared window, pod-set comparison, fresh-cluster
fallback, and the failure diagnostics. Reads the env vars raw rather than
through the existing helpers, since those helpers are what hide the missing
value. Production code (
client.go/env.go) is intentionally left unchangedso other tests and local demo runs keep their fallbacks.
test/integration/preflight/test.bats(new) — runs the guard withgo test -count=1(always re-queries live targets, never a cached result).Tagged
# bats test_tags=preflightso the main suite can exclude it..github/workflows/integration-testing.yaml(modified) — runs the guardas its own step before the main suite, so an incoherent environment aborts the
run early. The main suite still runs recursively (
bats -r test/integration)and uses
--filter-tags '!preflight'to skip the guard, so new top-level testdirs are still picked up automatically.
Testing
Ran the full pipeline locally against the demo target:
go vet ./...andgo build ./...— clean.(47 tests unfiltered, 46 filtered).
PROMETHEUS_URLunset, the guardt.Fatals before any query.263) and the full URL + sample-pod diagnostic — the exact bug the ticket
describes.